Skip to content

feat: Add counter metrics for consumed cycles#9922

Merged
michael-weigelt merged 1 commit intodfinity:masterfrom
dsarlis:dimitris/counter-metrics-cycles
May 5, 2026
Merged

feat: Add counter metrics for consumed cycles#9922
michael-weigelt merged 1 commit intodfinity:masterfrom
dsarlis:dimitris/counter-metrics-cycles

Conversation

@dsarlis
Copy link
Copy Markdown
Contributor

@dsarlis dsarlis commented Apr 17, 2026

Add counter versions of metrics for consumed cycles that are stored in ReplicatedState. The existing ones behave like gauges (so their values can go down when prepayments are made and up when refunds are issued) which makes it more challenging for consumers to build automated monitoring tools to perform aggregations over them. By having them monotonically increase, it's easier to calculate rates of change, show aggregates over time etc.

The key idea is to introduce a second map of <CyclesUseCase, NominalCycles> in the ReplicatedState that will only be updated once per use case: either at the payment stage if we know the precise amount or only at refund stage if a prepayment is made with an expected refund later. The second map is quite similar to the existing in all other aspects (how they are stored in checkpoints or how they are exposed as prometheus metrics) besides how the values are updated.

A new map is introduced to ease the transition as migrating from the old map to new is non-trivial given that a proper cutoff point needs to be introduced to handle outstanding callbacks that might have been created before the metric introduction. This is left for a follow-up if and when people decide to do it. The new map will be used in a follow up that will implement the new management canister endpoint to retrieve canister level metrics.

Additionally, the new metrics include the use case HttpsOutcalls in the canister level metrics as it's useful to determine how much each canister uses this feature. I've opted to not change existing metrics to do the same as it would make things less clean imo than the current approach -- a single specific API is used to perform this update in exactly one place where it's needed.

The changes in the PR are mostly driven by the addition of the new map of metrics, updates in protobuf files to store the new metrics, the changes to support having the HttpsOutcalls use case additionally included as well as some changes in tests to support the new metrics.

Copy link
Copy Markdown
Contributor Author

@dsarlis dsarlis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open points:

  1. Naming is something that I deliberately did not spend too much on. I can totally understand if a suffix _as_counters is not considered good enough and open to ideas for alternate names.
  2. In tests you'll notice that I've added a few places where I check the new counters additionally to old ones. One might wonder "what if we try to add such checks universally in ExecutionTest". I tried it and as it turns out it's not easy to make all tests work as some of them cut corners and you're not always able to do the check at "clear points", i.e. points where you know that there can be no outstanding callbacks (execute_all would be the best candidate and you get majority of tests to work but you still miss some special ones, highly concentrated in hypervisor_tests). I'd leave as an improvement if anyone wants to pick it up.

Comment thread rs/execution_environment/src/execution/response/tests.rs
Comment thread rs/replicated_state/src/canister_state/system_state.rs
Comment thread rs/replicated_state/src/canister_state/system_state.rs
@dsarlis dsarlis marked this pull request as ready for review April 17, 2026 11:02
@dsarlis dsarlis requested review from a team as code owners April 17, 2026 11:02
@github-actions
Copy link
Copy Markdown
Contributor

@basvandijk basvandijk added the security-review-passed IDX or InfraSec have concluded it's safe to run CI on the external PR. label Apr 17, 2026
Copy link
Copy Markdown
Contributor

@pierugo-dfinity pierugo-dfinity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving changes in rs/monitoring/metrics

@michael-weigelt michael-weigelt added this pull request to the merge queue May 5, 2026
Merged via the queue into dfinity:master with commit 0bf1d54 May 5, 2026
14 of 15 checks passed
@michael-weigelt michael-weigelt deleted the dimitris/counter-metrics-cycles branch May 5, 2026 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

@consensus external-contributor feat security-review-passed IDX or InfraSec have concluded it's safe to run CI on the external PR. @team-dsm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants